@bors try @rust-timer queue

Awaiting bors try build completion

⌛ Trying commit d0c63bc with merge 99e4ef7ba85e18068c4232297da28acab7db20f6...

☀️ Try build successful - checks-actions
Build commit: 99e4ef7ba85e18068c4232297da28acab7db20f6 (99e4ef7ba85e18068c4232297da28acab7db20f6)

Queued 99e4ef7ba85e18068c4232297da28acab7db20f6 with parent ffe5288, future comparison URL.

Finished benchmarking try commit (99e4ef7ba85e18068c4232297da28acab7db20f6): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-perf

It looks like adding a Lrc didn't result in any performance impact.

@bors r+

📌 Commit d0c63bc has been approved by Aaron1011

@bors rollup-

@Aaron1011
So, #78712 introduces one more case of converting a lazy token stream into a non-lazy one, and I started thinking about using an enum again.

The second reason for introducing an enum is that using None (of Option<LazyTokenStream>) in #77255 as both an unknown and as an empty stream feels sketchy, although I can't give an example where it would be incorrect.
Is this case intended for private visibilities specifically?

The problem is that the requirement sizeof(Option<LazyTokenStream>) <= 8 byte is significant for performance but very restrictive, if LazyTokenStream is made into enum, then you can't add even an empty variant without growing its size!

struct S1(Box<u8>);
struct S2(Option<Box<u8>>);
enum S3 { None, Some(Box<u8>) }
enum S4 { None, Empty, Some(Box<u8>) }
struct S5(Option<S3>);
struct S6(Option<S4>);

[src/main.rs:11] size_of::<S1>() = 8
[src/main.rs:12] size_of::<S2>() = 8
[src/main.rs:13] size_of::<S3>() = 8
[src/main.rs:14] size_of::<S4>() = 16
[src/main.rs:15] size_of::<S5>() = 16
[src/main.rs:16] size_of::<S6>() = 16

Perhaps if 16 byte is significantly better in terms of allocations and cloning, it will compensate... I need to check.

Since Lazytokenstream is heap allocated, you might consider bit packing the discriminant (by increasing alignment slightly); there's some helpers available in rustc data structures to do so somewhat easily. Unfortunately it's unlikely to be an ergonomic thing as you definitely lose matching and such.

Some statistics of token stream lengths in fn collect_tokens collected from bootstrapping rustc and stdlib:

  1  377069
  4   45481
  7   27001
 13   15311
 10   13782
 17   11456
  5   11309
  3   10065
  9    8041
  2    5961
  6    5295
 16    2672
  8    2079
 12    1692
 18    1679
 11    1349
  0    1300
 14    1243
 20    1003
 21     988

So, optimizing for 1-token streams is important, and optimizing for 0-token streams is not too important.